feat: add github stars, issues and created at to comparison page#2479
feat: add github stars, issues and created at to comparison page#2479ghostdevv merged 19 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Very nice addition.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three comparison facets— Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (composable)
participant Registry as npm Registry
participant Server as Nuxt Server (cached API)
participant GitHub as GitHub API
participant Cache as Server Cache
Client->>Registry: fetch package metadata (pkgData)
Registry-->>Client: pkgData (includes repository URL, time.created)
Client->>Client: detect GitHub repo from pkgData.repository
alt repo is GitHub
Client->>GitHub: fetch repo details (stars) or Server->>GitHub: via util
GitHub-->>Client: repo data (stars) or null
Client->>Server: GET /api/github/issues/{owner}/{repo}
Server->>Cache: lookup 'github-issue-count' for owner/repo
alt cache miss or stale
Server->>GitHub: search issues (fetchGitHubWithRetries, retry/backoff)
GitHub-->>Server: total_count or null
Server->>Cache: store result
end
Server-->>Client: { owner, repo, issues }
else non-GitHub
Client-->>Client: set github metadata to null/undefined
end
Client-->>Client: populate PackageComparisonData.metadata (createdAt, github.{stars,issues})
Client->>Client: compute facet values (githubStars, githubIssues, createdAt)
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/nuxt/composables/use-package-comparison.spec.ts (1)
196-233: Add a regression test for malformed GitHub payloads.Current coverage validates happy-path numeric responses, but not missing/invalid
stars/issuesfields. Add one case asserting the facet value isnull(not0) when the upstream payload shape is incomplete.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/nuxt/composables/use-package-comparison.spec.ts` around lines 196 - 233, Add a regression test in use-package-comparison.spec.ts that uses usePackageComparisonInComponent and the existing vi.stubGlobal('$fetch') pattern but returns malformed GitHub payloads (e.g., responses missing repo.stars or issues fields / non-numeric values) for the GitHub endpoints; after awaiting status 'success' call getFacetValues('githubStars')[0] and getFacetValues('githubIssues')[0] and assert each facet's value is null (not 0) and the facet status is the expected fallback (e.g., neutral/invalid) to ensure missing/invalid upstream shapes produce null facet values.test/nuxt/components/compare/FacetSelector.spec.ts (1)
35-37: Keep facet fixture labels aligned with real locale strings.The new fixture text drifts from production copy (notably
createdAt: “Created” vs “Created At”). This makes selector text assertions less representative of real UI output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/nuxt/components/compare/FacetSelector.spec.ts` around lines 35 - 37, The facet fixture labels in the test drift from production locale copy—update the fixture entry keys (githubStars, githubIssues, createdAt) so their label values exactly match the real locale strings (in particular change createdAt's label from "Created" to "Created At") and verify any other fixture labels mirror the app's locale source; locate these keys in the FacetSelector.spec.ts fixture and adjust the label values accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/composables/usePackageComparison.ts`:
- Around line 147-155: The current .then handlers in usePackageComparison.ts
coerce missing/malformed GitHub metrics to 0 (via `|| 0`), which hides unknown
values; update the .then callbacks on the stars and issues $fetch calls so they
return null when the API payload doesn't include a numeric value (e.g., change
the arrow results for the repo stars and issues to return null for missing
values instead of 0), and keep the existing .catch(() => null). Locate the two
$fetch chains guarded by isGitHub and repoInfo (the calls that currently use
`res?.repo.stars || 0` and `res?.issues || 0`) and replace those expressions
with a check that yields null for absent/non-numeric metrics (so downstream
comparison logic can handle "unknown" rather than 0).
In `@i18n/schema.json`:
- Around line 3985-4019: The added facet keys use snake_case and must be renamed
to match the project's camelCase convention to avoid lookup mismatches: change
"github_stars" -> "githubStars", "github_issues" -> "githubIssues", and
"created_at" -> "createdAt" in the schema object (keeping their inner structure
and additionalProperties: false intact); also search for any other references to
these snake_case keys and update them to the camelCase identifiers used at
runtime so labels/descriptions resolve correctly.
In `@server/api/github/issues/`[owner]/[repo].get.ts:
- Around line 35-37: The call to $fetch<GitHubSearchResponse>(url, { headers:
GITHUB_HEADERS }) must include authentication and a request timeout to avoid
GitHub Search API rate limits; update the $fetch invocation that returns data to
add an Authorization header (bearer token read from environment/config and
merged into GITHUB_HEADERS) and a reasonable timeout option (e.g., a few
seconds) so requests fail fast and are retried/handled upstream; ensure the
token usage is conditional (fall back safely if absent) and propagate
errors/timeouts from this call so callers of this endpoint can handle
403/timeout responses.
---
Nitpick comments:
In `@test/nuxt/components/compare/FacetSelector.spec.ts`:
- Around line 35-37: The facet fixture labels in the test drift from production
locale copy—update the fixture entry keys (githubStars, githubIssues, createdAt)
so their label values exactly match the real locale strings (in particular
change createdAt's label from "Created" to "Created At") and verify any other
fixture labels mirror the app's locale source; locate these keys in the
FacetSelector.spec.ts fixture and adjust the label values accordingly.
In `@test/nuxt/composables/use-package-comparison.spec.ts`:
- Around line 196-233: Add a regression test in use-package-comparison.spec.ts
that uses usePackageComparisonInComponent and the existing
vi.stubGlobal('$fetch') pattern but returns malformed GitHub payloads (e.g.,
responses missing repo.stars or issues fields / non-numeric values) for the
GitHub endpoints; after awaiting status 'success' call
getFacetValues('githubStars')[0] and getFacetValues('githubIssues')[0] and
assert each facet's value is null (not 0) and the facet status is the expected
fallback (e.g., neutral/invalid) to ensure missing/invalid upstream shapes
produce null facet values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d78fa7b1-bac9-47b1-a5ad-cecade8af8dc
📒 Files selected for processing (9)
app/composables/useFacetSelection.tsapp/composables/usePackageComparison.tsapp/utils/compare-scatter-chart.tsi18n/locales/en.jsoni18n/schema.jsonserver/api/github/issues/[owner]/[repo].get.tsshared/types/comparison.tstest/nuxt/components/compare/FacetSelector.spec.tstest/nuxt/composables/use-package-comparison.spec.ts
app/composables/useFacetSelection.ts
Outdated
| description: t(`compare.facets.items.created_at.description`), | ||
| chartable: false, | ||
| chartable_scatter: false, | ||
| formatter: v => new Date(v).toLocaleDateString(), |
There was a problem hiding this comment.
The formatter for createdAt is unused and should be removed.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/api/github/contributors-evolution/`[owner]/[repo].get.ts:
- Around line 30-32: The fetch call to GitHub uses
fetchGitHubWithRetries<GitHubContributorStats[]> but omits a request timeout, so
update the options passed to fetchGitHubWithRetries (the call that assigns to
data) to include a timeout (e.g. timeout: 10000) alongside maxAttempts: 6; this
ensures each retry attempt will fail fast instead of potentially blocking
indefinitely. Target the invocation of fetchGitHubWithRetries in the handler for
contributors-evolution ([owner]/[repo].get.ts) and add the timeout property to
the NitroFetchOptions object.
In `@server/utils/github.ts`:
- Around line 12-13: After destructuring maxAttempts from options (const {
maxAttempts = 3, ...fetchOptions } = options) ensure it is clamped/validated to
at least 1 before any retry loop runs (e.g. replace or follow the destructuring
with something like maxAttempts = Math.max(1, Number(maxAttempts || 0))). Apply
the same guard to the other destructurings that expose maxAttempts (the
occurrences near the other two diffs) so the retry loops using delayMs and
maxAttempts cannot be skipped when maxAttempts <= 0.
- Around line 23-28: The current merge in the $fetch.raw call loses headers when
fetchOptions.headers is a Headers instance or array-of-tuples; update the header
merge logic used before calling $fetch.raw (where defaultHeaders and
fetchOptions.headers are combined) to normalize HeadersInit into a single
Headers and then copy entries so all forms are supported: create a Headers from
defaultHeaders, then if fetchOptions.headers is a Headers, iterate its entries
and set them, if it's an array of tuples iterate and set, and if it's a plain
object iterate its keys and set; finally pass the resulting Headers (or a plain
object built from it) into $fetch.raw so callers using Headers, tuple arrays, or
objects all have their headers preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0be4d085-3032-4fcb-96d0-0c61ce803df9
📒 Files selected for processing (3)
server/api/github/contributors-evolution/[owner]/[repo].get.tsserver/api/github/issues/[owner]/[repo].get.tsserver/utils/github.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- server/api/github/issues/[owner]/[repo].get.ts
| const data = await fetchGitHubWithRetries<GitHubContributorStats[]>(url, { | ||
| maxAttempts: 6, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the file under review
fd "contributors-evolution" -t fRepository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
# Search for the fetchGitHubWithRetries function definition
rg "fetchGitHubWithRetries" -t ts -t tsx --max-count=20 -A 5 -B 2Repository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Look for function implementation and type definitions
rg "function fetchGitHubWithRetries|const fetchGitHubWithRetries" -t ts -t tsx -A 15Repository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Find the specific file being reviewed
find . -name "*.get.ts" -path "*contributors-evolution*"Repository: npmx-dev/npmx.dev
Length of output: 125
🏁 Script executed:
# Search for fetchGitHubWithRetries without file type flags
rg "fetchGitHubWithRetries" -A 10 -B 2Repository: npmx-dev/npmx.dev
Length of output: 2684
🏁 Script executed:
# Also search for timeout-related configuration in GitHub API utilities
rg "timeout" --max-count=30Repository: npmx-dev/npmx.dev
Length of output: 6996
🏁 Script executed:
# Read the full implementation of fetchGitHubWithRetries
head -100 server/utils/github.tsRepository: npmx-dev/npmx.dev
Length of output: 1485
🏁 Script executed:
# Look for GitHubFetchOptions type definition
rg "GitHubFetchOptions" -A 8 -B 2Repository: npmx-dev/npmx.dev
Length of output: 941
Add an explicit timeout for the GitHub call.
The fetchGitHubWithRetries function supports timeout via NitroFetchOptions. Line 30–32 configures 6 retry attempts but lacks a request timeout, leaving each attempt vulnerable to indefinite blocking. The same pattern is used elsewhere in the codebase with timeout: 10000 (see server/api/github/issues/[owner]/[repo].get.ts).
Suggested fix
const data = await fetchGitHubWithRetries<GitHubContributorStats[]>(url, {
maxAttempts: 6,
+ timeout: 10_000,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const data = await fetchGitHubWithRetries<GitHubContributorStats[]>(url, { | |
| maxAttempts: 6, | |
| }) | |
| const data = await fetchGitHubWithRetries<GitHubContributorStats[]>(url, { | |
| maxAttempts: 6, | |
| timeout: 10_000, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/api/github/contributors-evolution/`[owner]/[repo].get.ts around lines
30 - 32, The fetch call to GitHub uses
fetchGitHubWithRetries<GitHubContributorStats[]> but omits a request timeout, so
update the options passed to fetchGitHubWithRetries (the call that assigns to
data) to include a timeout (e.g. timeout: 10000) alongside maxAttempts: 6; this
ensures each retry attempt will fail fast instead of potentially blocking
indefinitely. Target the invocation of fetchGitHubWithRetries in the handler for
contributors-evolution ([owner]/[repo].get.ts) and add the timeout property to
the NitroFetchOptions object.
Refactor header handling in fetch request to use Headers object.
ghostdevv
left a comment
There was a problem hiding this comment.
I wonder if we should open some PRs to ungh for the missing parts, or continue to fetch directly - WDYT?
Would improve ergonomics having it in ungh itself for sure. Can't provide an estimate yet though, of how fast I can do that |
alexdln
left a comment
There was a problem hiding this comment.
lgtm too, thanks for updates 🤍
🔗 Linked issue
Resolves #2460
🧭 Context
In #2460, additional facets were requested including:
This PR implements these facets.
📚 Description
Created atfacet was implemented using the information already available information inpkgData.GitHub Starsfacet was implemented by introducing anotherfetchcall in the existingPromise.allfor additional data fetching for the comparison. It uses theparseRepositoryInfofunction fromshared/utils/git-providers.tsto ensure that the fetch is only run for GitHub repositories. It then extracts the owner and repo name from the URL stored in thepackage.jsonof the package being compared. Afterwards, it makes a direct fetch toungh.cc/repos/owner/repoand extracts{ repo: { stars: number } }from the returned payload.GitHub Issuesfacet was implemented by introducing another API endpoint inserver/api/github/issues/[owner]/[repo].get.ts. First, with help fromparseRepositoryInfo, the provider is checked, then a fetch against that API endpoint is made. The API endpoint itself queries GitHub’s REST API to get the number of open issues fromhttps://api.github.com/search/issues?q=repo:${owner}/${repo}%20is:issue is:open&per_page=1. Those issues are then returned to thePromise.alland used.Localization keys were added to
en.json, and tests were implemented for the newly introduced facets.📷 Screenshots